Skip to content

Remove R/rpy2 dependency; replace with cffdrs_py#5176

Merged
conbrad merged 24 commits intomainfrom
task/cffdrs-py-no-r
Mar 10, 2026
Merged

Remove R/rpy2 dependency; replace with cffdrs_py#5176
conbrad merged 24 commits intomainfrom
task/cffdrs-py-no-r

Conversation

@conbrad
Copy link
Collaborator

@conbrad conbrad commented Mar 2, 2026

Replaces all R/rpy2 calls in app/fire_behaviour/cffdrs.py with the pure-Python cffdrs_py package, eliminating the R runtime as a dependency.

Changes:

  • Rewrote all FWI and FBP wrappers in cffdrs.py to use cffdrs_py submodule imports
  • Removed CFFDRS singleton, r_importer.py, and CFFDRS pre-warming from startup
  • Removed rpy2 from pyproject.toml; added cffdrs git dependency (pinned to commit)
  • Removed R installation from base image Dockerfile, OpenShift build.yaml, and local dev Dockerfile
  • Improved None guards on bui_calc, initial_spread_index, fire_weather_index, and crown_fraction_burned; fixed drought_code silently substituting rh=0 when RH is None; fixed
    length_to_breadth_ratio returning rather than raising CFFDRSException; added per-row error handling to hourly_fine_fuel_moisture_code
  • Updated 32 fba_calc tests (removed CFFDRS.instance() pre-warming); deleted test_r.py

Behavioural notes:

  • duff_moisture_code returns 0.0 (not ~0.256) when temperature < 1.1°C — cffdrs_py clamps temp to -1.1 before computing the drying rate (Eq. 16), zeroing it out. The R implementation did not apply this clamp.
  • cbh uses a fuel type default if it is None, this was handled internally before in the R library.

Closes #1926

Test Links:
Landing Page
MoreCast
Percentile Calculator
C-Haines
FireCalc
FireCalc bookmark
Auto Spatial Advisory (ASA)
HFI Calculator
SFMS Insights
Fire Watch

@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 74.07407% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.06%. Comparing base (8231082) to head (c9c728a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../packages/wps-api/src/app/fire_behaviour/cffdrs.py 73.01% 20 Missing and 14 partials ⚠️
...pi/src/app/auto_spatial_advisory/critical_hours.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5176      +/-   ##
==========================================
- Coverage   69.06%   69.06%   -0.01%     
==========================================
  Files         394      393       -1     
  Lines       16418    16337      -81     
  Branches     1846     1820      -26     
==========================================
- Hits        11339    11283      -56     
+ Misses       4495     4479      -16     
+ Partials      584      575       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@conbrad conbrad requested review from brettedw and dgboss March 2, 2026 22:45
@brettedw
Copy link
Collaborator

brettedw commented Mar 5, 2026

cbh uses a fuel type default if it is None, this was handled internally before in the R library.

Where does this happen? I think cffdrs_py still handles it internally

@conbrad
Copy link
Collaborator Author

conbrad commented Mar 5, 2026

cbh uses a fuel type default if it is None, this was handled internally before in the R library.

Where does this happen? I think cffdrs_py still handles it internally

backend/packages/wps-api/src/app/routers/fba_calc.py

@brettedw
Copy link
Collaborator

brettedw commented Mar 6, 2026

cbh uses a fuel type default if it is None, this was handled internally before in the R library.

Where does this happen? I think cffdrs_py still handles it internally

backend/packages/wps-api/src/app/routers/fba_calc.py

Took me awhile to figure out what was happening here, I was confused because R doesn't have fuel type default handling in those functions either, but it comes down to Python vs R math operations on NULL/None. Python will error, R won't.

Non-C6 fuel: ROS still comes from RSS (rate_of_spread.r (line 173)), so you often still get a numeric ROS.
C6 fuel: ROS depends on CFB/RSO, so it typically becomes NA (rate_of_spread.r (line 174)).

This is why C6 fuel type has built in 2 and 7m CBH on the front end

Regardless, all the calculations in FireCalc look the same to me

@conbrad conbrad requested review from brettedw and dgboss March 10, 2026 00:35
Copy link
Collaborator

@dgboss dgboss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 So nice to finally remove R! We've been talking about this since I joined the team I think.

@brettedw
Copy link
Collaborator

Do you think we should remove R installation and setup from mac.sh and put it elsewhere? It's not necessary for local running and builds anymore, but still nice to have it installed as a dev for comparing R to Python CFFDRS

@sonarqubecloud
Copy link

@conbrad
Copy link
Collaborator Author

conbrad commented Mar 10, 2026

remove R install from mac setup

Done in: c9c728a

crown_base_height=requested_station.crown_base_height,
crown_base_height=requested_station.crown_base_height
if requested_station.crown_base_height is not None
else FUEL_TYPE_DEFAULTS[requested_station.fuel_type]["CBH"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cffdrs_py has crown_base_height and crown_fuel_load. Wondering if we should start using those vs our FUEL_TYPE_DEFAULTS. Or if that's a later ticket. PC, PDF and CC might be harder to sort out

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah let's do a follow up if we didn't use that before from the R package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@conbrad conbrad requested a review from brettedw March 10, 2026 21:26
Copy link
Collaborator

@brettedw brettedw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@conbrad conbrad temporarily deployed to production March 10, 2026 21:31 Inactive
@conbrad conbrad merged commit 65ad04a into main Mar 10, 2026
28 checks passed
@conbrad conbrad deleted the task/cffdrs-py-no-r branch March 10, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CFFDRS API : Get R the F out of our S (Reference ticket)

3 participants